Skip to content

[Relax][TVMScript] Print ExternFunc struct_info when non-default#19416

Merged
tlopex merged 4 commits intoapache:mainfrom
cchung100m:refactor_externFunc
Apr 18, 2026
Merged

[Relax][TVMScript] Print ExternFunc struct_info when non-default#19416
tlopex merged 4 commits intoapache:mainfrom
cchung100m:refactor_externFunc

Conversation

@cchung100m
Copy link
Copy Markdown
Contributor

Summary

  1. Add HasDefaultExternFuncStructInfo helper to detect default FuncStructInfo for extern functions.

  2. Update relax::ExternFunc printer to:

  • emit global_symbol using the correct AccessPath attribute key,
  • conditionally include struct_info only when it differs from the default inferred-by-sinfo-args derive function,
  • use a variadic args array instead of a single positional literal to prepare the ExternFunc call.
  1. This reduces noisy/redundant output when printing ExternFunc nodes while preserving explicit struct_info when it conveys meaningful information.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances the Relax printer by introducing a helper function, HasDefaultExternFuncStructInfo, which allows the printer to conditionally omit structural information for external functions when they use the default inference function. This change results in cleaner and more concise printed output. Feedback was provided to optimize the new helper function by marking it as static and caching the global function lookup to avoid redundant registry queries during the printing process.

Comment thread src/script/printer/relax/function.cc Outdated
@cchung100m cchung100m force-pushed the refactor_externFunc branch from 092a469 to b8b10de Compare April 17, 2026 13:29
@cchung100m cchung100m force-pushed the refactor_externFunc branch from b8b10de to ea93e01 Compare April 17, 2026 13:31
@cchung100m cchung100m marked this pull request as ready for review April 17, 2026 14:37
@cchung100m cchung100m marked this pull request as draft April 17, 2026 14:41
@cchung100m cchung100m marked this pull request as ready for review April 17, 2026 16:16
@cchung100m
Copy link
Copy Markdown
Contributor Author

Hi @tlopex @mshr-h ,

This PR is trying to finish TODO task. Any suggestions would be appreciated if you are available. 😄

Copy link
Copy Markdown
Member

@tlopex tlopex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! @cchung100m

  1. Should add test coverage in tests/python/relax/test_tvmscript_printer_relax.py. At minimum, this should include:
  • one case with non-default struct_info (for example, with purity=True or explicit params / ret)
  • one round-trip test
  1. Maybe update the PR title from Unity to Relax, which I think is better

@cchung100m cchung100m changed the title [Unity][TVMScript] Print ExternFunc struct_info when non-default [Relax][TVMScript] Print ExternFunc struct_info when non-default Apr 18, 2026
@cchung100m cchung100m marked this pull request as draft April 18, 2026 05:50
@cchung100m cchung100m marked this pull request as ready for review April 18, 2026 08:28
@cchung100m
Copy link
Copy Markdown
Contributor Author

Hi @tlopex

Thanks for the prompt reply and I have updated the part you mentioned.

@tlopex tlopex merged commit ca68bef into apache:main Apr 18, 2026
10 checks passed
@cchung100m
Copy link
Copy Markdown
Contributor Author

Thanks to @tlopex 😄

@cchung100m cchung100m deleted the refactor_externFunc branch April 19, 2026 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants